Fix label events overwriting CI check statuses#466
Conversation
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/retest.yaml">
<violation number="1" location=".github/workflows/retest.yaml:11">
P1: Security: Missing authorization check on commenter. Any GitHub user who can comment on a PR can trigger CI reruns or dispatch fresh workflow runs. The existing `reset-kelos-worker.yaml` workflow in this repo properly checks that the commenter has admin permission via `getCollaboratorPermissionLevel` before proceeding — this workflow should do the same (or at minimum check for `write` permission).</violation>
<violation number="2" location=".github/workflows/retest.yaml:23">
P2: Missing guard for empty `RUN_ID`. If no CI run is found for the HEAD SHA (e.g., CI hasn't run yet or force-push changed the SHA), `RUN_ID` is empty, `gh run view` fails silently (suppressed by `2>/dev/null`), and the script falls through to dispatching a fresh CI run without any indication of what happened. Add an explicit check after the `RUN_ID` assignment.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
/retest |
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/retest.yaml">
<violation number="1" location=".github/workflows/retest.yaml:34">
P2: When CI is still in-progress, `conclusion` is empty, so this falls through to dispatch a brand-new parallel CI run. Consider checking for an in-progress run (e.g., `status == "in_progress"` or empty conclusion) and either skipping the dispatch or informing the user that CI is already running.</violation>
<violation number="2" location=".github/workflows/retest.yaml:37">
P2: The `else` branch here is an exact duplicate of the outer `else` block (no `RUN_ID` case). Restructure to avoid duplication — e.g., only special-case the `failure` rerun, and fall through to a single dispatch block otherwise.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
.github/workflows/retest.yaml
Outdated
| CONCLUSION=$(gh run view "$RUN_ID" --json conclusion -q .conclusion) | ||
| if [ "$CONCLUSION" = "failure" ]; then | ||
| gh run rerun "$RUN_ID" --failed | ||
| else |
There was a problem hiding this comment.
P2: The else branch here is an exact duplicate of the outer else block (no RUN_ID case). Restructure to avoid duplication — e.g., only special-case the failure rerun, and fall through to a single dispatch block otherwise.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/retest.yaml, line 37:
<comment>The `else` branch here is an exact duplicate of the outer `else` block (no `RUN_ID` case). Restructure to avoid duplication — e.g., only special-case the `failure` rerun, and fall through to a single dispatch block otherwise.</comment>
<file context>
@@ -13,16 +13,32 @@ jobs:
+ CONCLUSION=$(gh run view "$RUN_ID" --json conclusion -q .conclusion)
+ if [ "$CONCLUSION" = "failure" ]; then
+ gh run rerun "$RUN_ID" --failed
+ else
+ HEAD_REF=$(gh pr view "$PR_NUMBER" --json headRefName -q .headRefName)
+ OK_TO_TEST=$(gh pr view "$PR_NUMBER" --json labels -q '[.labels[].name] | any(. == "ok-to-test")')
</file context>
| HEAD_SHA=$(gh pr view "$PR_NUMBER" --json headRefOid -q .headRefOid) | ||
| RUN_ID=$(gh run list --workflow=ci.yaml --commit="$HEAD_SHA" --limit=1 --json databaseId -q '.[0].databaseId') | ||
| if [ -n "$RUN_ID" ]; then | ||
| CONCLUSION=$(gh run view "$RUN_ID" --json conclusion -q .conclusion) |
There was a problem hiding this comment.
P2: When CI is still in-progress, conclusion is empty, so this falls through to dispatch a brand-new parallel CI run. Consider checking for an in-progress run (e.g., status == "in_progress" or empty conclusion) and either skipping the dispatch or informing the user that CI is already running.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/retest.yaml, line 34:
<comment>When CI is still in-progress, `conclusion` is empty, so this falls through to dispatch a brand-new parallel CI run. Consider checking for an in-progress run (e.g., `status == "in_progress"` or empty conclusion) and either skipping the dispatch or informing the user that CI is already running.</comment>
<file context>
@@ -13,16 +13,32 @@ jobs:
- if [ "$CONCLUSION" = "failure" ]; then
- gh run rerun "$RUN_ID" --failed
+ if [ -n "$RUN_ID" ]; then
+ CONCLUSION=$(gh run view "$RUN_ID" --json conclusion -q .conclusion)
+ if [ "$CONCLUSION" = "failure" ]; then
+ gh run rerun "$RUN_ID" --failed
</file context>
| CONCLUSION=$(gh run view "$RUN_ID" --json conclusion -q .conclusion) | |
| CONCLUSION=$(gh run view "$RUN_ID" --json conclusion -q .conclusion) | |
| STATUS=$(gh run view "$RUN_ID" --json status -q .status) | |
| if [ "$STATUS" = "in_progress" ] || [ "$STATUS" = "queued" ] || [ "$STATUS" = "waiting" ]; then | |
| echo "CI run $RUN_ID is still $STATUS — skipping retest" | |
| exit 0 | |
| fi |
4ddb265 to
d9d3321
Compare
Remove labeled from CI triggers to prevent label events from reporting SKIPPED and overwriting previous SUCCESS checks. Add workflow_dispatch trigger and a /retest comment workflow to re-trigger CI on demand.
d9d3321 to
34d2ff3
Compare
Summary
labeledfrom CIpull_requesttriggers to prevent label events from reporting jobs as SKIPPED, which overwrites previous SUCCESS checksworkflow_dispatchtrigger withok-to-testinput for manual CI runs/retestcomment workflow that re-runs only failed jobs or dispatches a fresh CI run with current label stateTest plan
/reteston a failed run → only failed jobs re-run/reteston a passing run → fresh CI dispatched with current label state🤖 Generated with Claude Code
Summary by cubic
Stops label changes from re-triggering CI and overwriting successful checks with SKIPPED. Adds a safe, permission-gated way to rerun CI on demand.
Bug Fixes
New Features
Written for commit 34d2ff3. Summary will update on new commits.